Refactor analysis to decouple I/O from core logic#8426
Conversation
Splits `Commands.ml` into a pure layer that returns OCaml values (option, list, typed records like Protocol.hover, Protocol.signatureHelp, Protocol.completionItem) and a new `analysis/src/Cli.ml` that does the stringify-and-print step. `analysis/bin/main.ml` now dispatches to `Cli.*`, while the LSP server consumes `Commands.*` directly. Makes the parsers accept source strings: `res_driver` gains `parse_interface_from_source` alongside the existing `parse_implementation_from_source`
| let emit e = | ||
| let sortedTokens = | ||
| e.tokens | ||
| |> List.sort (fun (l1, c1, _, _) (l2, c2, _, _) -> | ||
| if l1 = l2 then compare c1 c2 else compare l1 l2) | ||
| in | ||
| let buf = Buffer.create 1 in | ||
| sortedTokens |> List.iter (fun t -> e |> emitToken buf t); | ||
| let arrays = sortedTokens |> List.filter_map (fun t -> e |> emitToken t) in | ||
| Array.concat arrays |
There was a problem hiding this comment.
Instead returning a string "[0,1,2,3]" return a int array https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#semanticTokens
| let fields = | ||
| [("range", Some (Protocol.stringifyRange range))] | ||
| @ | ||
| match placeholderOpt with | ||
| | None -> [] | ||
| | Some s -> [("placeholder", Some (Protocol.wrapInQuotes s))] | ||
| in |
There was a problem hiding this comment.
I believe this was incorrect. Prepare rename should return Range | { range: Range, placeholder: string } | { defaultBehavior: boolean } | null (https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_prepareRename)
placeholderOpt is an option, so if it's None, the placeholder field doesn't exist; however, { range: Range, placeholder: string } is expected.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ccb7ecb9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The previous implementation used the source byte length as both line and character values for the end of the format range, which was incorrect. Replace it with a helper that computes the actual final line index and character offset by splitting on newlines. Signed-Off-By: Your Name <email>
2 participants
Splits
Commands.mlinto a pure layer that returns OCaml values (option, list, typed records like Protocol.hover,Protocol.signatureHelp, Protocol.completionItem) and a new
analysis/src/Cli.mlthat does the stringify-and-print step.analysis/bin/main.mlnow dispatches toCli.*, while the LSP server consumesCommands.*directly.Makes the parsers accept source strings:
res_drivergainsparse_interface_from_sourcealongside the existingparse_implementation_from_sourceCherry-pick of #8425